Add per-tool toggles and show what each service exposes - #190
Open
Arsey wants to merge 6 commits into
Open
Conversation
A JSON-encoded set of disabled tool names is persisted the same way as trusted clients and pushed into ServerNetworkManager the same way as service bindings. ListTools omits disabled tools and CallTool rejects them, so a client holding a stale list still cannot invoke one. Connected clients get tools/listChanged on every change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLDiNzrDds62dnD9NoCEJV
Each write to ServerController.disabledTools fired an unstructured Task carrying a snapshot to the actor. Two rapid writes could reach the actor out of order, and the actor's equality guard would accept the stale snapshot, leaving live ListTools/CallTool enforcement diverged from the persisted value until the next toggle. A monotonic generation counter is now attached to each push; the actor discards any delivery whose generation is not newer than the last one it applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLDiNzrDds62dnD9NoCEJV
Each service lists its tools with human-readable titles, descriptions, and a Read-only badge from the tool annotations, plus a switch per tool backed by the disabled-tools set. The service master toggle is the same binding the menu uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLDiNzrDds62dnD9NoCEJV
Two Important findings from the Task 2 review, both in
App/Views/ServicesSettingsView.swift:
1. `.accessibilityElement(children: .combine)` on each tool row merged
the label and the interactive Toggle into a single non-actionable AX
element, so VoiceOver users could not flip a tool's switch. Removed
the combine modifier and gave both the per-tool and per-service
Toggle real accessibility labels (tool title / service name) with
`.labelsHidden()` to keep the visual layout unchanged while exposing
a named, actionable control to assistive technology.
2. Freshly rendered/scrolled switch rows could transiently paint "off"
while the persisted value was "on" (view identity was reused across
rows with different underlying state). Per the controller's ruling,
mitigated by keying each switch's view identity to its current value
via `.id("\(name)-\(currentValue)")`, so a newly materialized row
constructs its switch with the right state.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QLDiNzrDds62dnD9NoCEJV
Hovering a service row now shows what enabling it exposes, using the tool annotation titles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLDiNzrDds62dnD9NoCEJV
…tion The Settings service toggle (ServicesSettingsView.serviceBinding) wrote config.binding.wrappedValue directly, bypassing the activation flow that the menu path (ServiceToggleView) already performs. Enabling a service like Calendar from Settings never triggered the macOS permission prompt, and there was no revert-to-off if activation failed or was denied. It also never pushed the updated bindings to ServerNetworkManager, so connected MCP clients were not notified via tools/listChanged when a service was toggled from Settings (only the menu scene's ContentView .onChange did this). Add ServerController.setService(_:enabled:), which updates the binding, calls config.service.activate() when enabling an unactivated service (reverting on failure), and pushes currentServiceBindings to the network manager so notifyToolListChanged fires for connected clients. Route ServicesSettingsView.serviceBinding's setter through it, removing the now-redundant manual objectWillChange.send() from the view. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLDiNzrDds62dnD9NoCEJV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enabling a service is all-or-nothing, and nothing in the UI says what it grants. Toggling Calendar on exposes event creation as well as reading — the user can't see that, and can't allow one without the other. For an app whose job is handing personal data to AI clients, that's too coarse.
This adds per-tool control, enforced by the server:
tools/listand rejectstools/call— a client holding a stale list still can't invoke a disabled tool. Changes push to connected clients viatools/listChanged. Updates to the network actor carry a generation counter so out-of-order delivery can't apply a stale set.Default state (nothing disabled) serves the identical tool list as before.
Conflicts with #183 on one tooltip line in
ServiceToggleView— trivial to resolve whichever lands second.Test plan
calendars_list→ absent fromtools/list,tools/callreturns the disabled error → persists across relaunch → restore returns the baseline byte-identicalxcodebuild buildsucceedstools/listrefresh in a live Claude Desktop session at the moment a switch is clickedKnown issue
Freshly rendered switch rows in the pane can transiently paint the wrong position (state is never wrong, repaint corrects it). Mitigated with value-keyed identity; the full fix is moving the disabled set to a published property, which I kept out of scope here.